Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interact menu - Convert 'use list menu' setting to 'use wheel menu' #6656

Closed
wants to merge 3 commits into from

Conversation

alganthe
Copy link
Contributor

When merged this pull request will:

  • Convert "use list menu" setting to "use wheel menu" which is false by default.

Lists are superior, can display more elements at once and are easier to read.

@Drofseh
Copy link
Contributor

Drofseh commented Oct 28, 2018

I do think circle sounds better than wheel in English.
"Use circular menu"

@alganthe
Copy link
Contributor Author

it's all synonyms, radial sound good too but it might be harder to translate, so I kept it simple

@Drofseh
Copy link
Contributor

Drofseh commented Oct 28, 2018

It doesn't work grammatically in English because wheel is a noun.
It would have to be "Use wheel shaped menu".
Circular is an adjective, so "Use circular menu" works.

Copy link
Contributor

@Drofseh Drofseh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@dedmen dedmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I don't like this change but oh well... I don't like many things recently.

@PabstMirror
Copy link
Contributor

Is this a real PR or a shitpost?

@alganthe
Copy link
Contributor Author

alganthe commented Oct 30, 2018

That's not very nice pabst, and yes this is a real PR, list are superior to radial, they can display more entries with longer text and are easier to navigate, as such they should be made the default behavior of the interaction menu.

@bux
Copy link
Member

bux commented Oct 30, 2018

Let's not change years old defaults.

Copy link
Contributor

@thojkooi thojkooi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes default settings and breaks client configured preferences.

@jonpas
Copy link
Member

jonpas commented Nov 18, 2018

I am undecided, if most agree it should not be merged, let's just close it.

@PabstMirror
Copy link
Contributor

Sorry for my rude comment
#6609 was just merged
so if we wanted to change any defaults now would be a good time
but I still think keeping defaults the same is for the best

@commy2
Copy link
Contributor

commy2 commented Dec 8, 2019

I would've just changed value = 0; to value = 1;

@Drofseh
Copy link
Contributor

Drofseh commented Dec 8, 2019

I would still like to see this changed.
List is vastly superior to radial for ease of use.
Sub-nodes appear in more predictable places, you can have a much larger number of sub-nodes before you run into space constraints, and you can have longer node name strings before the end of the string runs into another node.

@jonpas
Copy link
Member

jonpas commented Dec 17, 2019

I am still in favour of this change, and @commy2's comment:

I would've just changed value = 0; to value = 1;

@jonpas jonpas added status/discussion kind/enhancement Release Notes: **IMPROVED:** labels Dec 17, 2019
@jonpas jonpas added this to the Backlog milestone Dec 17, 2019
@kymckay
Copy link
Member

kymckay commented Dec 17, 2019

I personally think list should be the default, however would just change the default setting value rather than create a new setting for backwards compatibility.

@jonpas
Copy link
Member

jonpas commented Apr 20, 2021

Replaced by #8217.

@jonpas jonpas closed this Apr 20, 2021
@LinkIsGrim LinkIsGrim removed this from the Backlog milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:** status/discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants